Skip to content

Conversation

@wonderwhy-er
Copy link
Owner

@wonderwhy-er wonderwhy-er commented Oct 1, 2025

Summary by CodeRabbit

  • New Features

    • Improved directory listings: top-level directories show all items, while nested directories are limited to 100 items to improve performance and readability.
    • When limits are reached, a clear warning indicates how many items were hidden and the total item count in that directory.
  • Documentation

    • Updated guidance to explain the item cap for nested directories, the warning format users will see, and the rationale to prevent context overload.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Adds documentation in src/server.ts about context overflow protection for ListDirectory. Implements a cap in src/tools/filesystem.ts to limit items shown for nested directories to 100, adds warnings when items are hidden, and updates recursive traversal to distinguish top-level vs nested calls. No public APIs changed.

Changes

Cohort / File(s) Summary
Filesystem listing logic
src/tools/filesystem.ts
Add MAX_NESTED_ITEMS=100; extend listRecursive with isTopLevel flag; cap entries for non-top-level dirs; append [WARNING] line when items are hidden; pass isTopLevel=false on recursion; adjust initial invocation to listRecursive(..., true).
Documentation update
src/server.ts
Insert inline docs describing ListDirectory context overflow protection, item caps for nested directories, and warning format; no functional code changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Server
  participant Filesystem as listDirectory
  participant Helper as listRecursive

  User->>Server: Request directory listing
  Server->>Filesystem: listDirectory(path, depth)
  Filesystem->>Helper: listRecursive(path, depth, prefix, isTopLevel=true)

  alt Top-level directory
    Helper->>Helper: Read all entries (no cap)
  else Nested directory
    Helper->>Helper: Read entries\nIf count > 100, slice to first 100
    Note right of Helper: Append "[WARNING] ... X hidden of Y total"
  end

  loop For each subdirectory
    Helper->>Helper: listRecursive(subdir, depth-1, newPrefix, isTopLevel=false)
  end

  Helper-->>Filesystem: Formatted listing (with optional warnings)
  Filesystem-->>Server: Result
  Server-->>User: Response
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I hop through trees of files so deep,
Listing paths I dare not keep—
A hundred peeks, the rest I hide,
With warnings posted by the side.
Ears up, context safe and tight,
Thump-thump—clean listings, moonlit night. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately summarizes the primary change by stating that a limit is being added to subfolder listings to prevent context overload, which aligns with the implementation of MAX_NESTED_ITEMS and warning logic in nested directory handling.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch get-list-optimisation

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb8d28 and e1b8a8a.

📒 Files selected for processing (2)
  • src/server.ts (1 hunks)
  • src/tools/filesystem.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/tools/filesystem.ts (1)
test/test-home-directory.js (1)
  • entries (224-224)
🔇 Additional comments (9)
src/server.ts (1)

333-339: Clear and accurate documentation.

The CONTEXT OVERFLOW PROTECTION documentation accurately reflects the implementation in src/tools/filesystem.ts. The warning format example matches the actual output format, and the distinction between top-level (unlimited) and nested directories (100 item cap) is clearly explained.

src/tools/filesystem.ts (8)

897-898: Reasonable constant definition.

The MAX_NESTED_ITEMS = 100 limit is well-chosen for context management. The constant is appropriately scoped within the function.


899-899: Well-designed parameter addition.

The isTopLevel parameter with a default value of true maintains backward compatibility while enabling the new filtering behavior. The parameter name is clear and self-documenting.


912-921: Correct filtering logic.

The filtering implementation properly distinguishes between top-level (unlimited) and nested directories (capped at 100 items). The logic correctly:

  • Applies filtering only to nested directories with more than 100 items
  • Calculates the hidden count accurately
  • Defaults to showing all entries when filtering isn't needed

922-922: Essential loop modification.

Correctly changed the iteration from entries to entriesToShow to apply the filtering. This ensures that only the capped number of items are processed and added to results.


934-934: Correct recursive call update.

Passing isTopLevel = false ensures that all subdirectories are treated as nested and subject to the 100-item cap. This correctly implements the cascading filter behavior.


943-947: Clear and informative warning message.

The warning is appropriately placed after processing the filtered entries and provides helpful information:

  • Shows the exact number of hidden items
  • Indicates the total item count
  • Matches the format documented in src/server.ts

The conditional ensures warnings are only shown when items are actually filtered.


950-950: Explicit top-level designation.

Passing isTopLevel = true explicitly in the initial call improves code clarity, even though the default value would achieve the same result. This makes the intent clear and aids maintainability.


897-950: No test updates required

Existing tests only verify the presence of specific entries (e.g., using entries.some(…)) and do not assert on complete directory lengths, so capping nested listings at 100 items won’t break any current assertions.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@wonderwhy-er wonderwhy-er merged commit 6586aa2 into main Oct 1, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants